-
Notifications
You must be signed in to change notification settings - Fork 69
konflux build: add openblas-thread lib for numpy wheel #1066
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughFinal image package list and RPM lockfile updated: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Containerfile`:
- Around line 95-98: Remove the temporary NumPy verification RUN block that
echoes and runs python3.12 -c "import numpy..." from the Containerfile; if you
must keep a temporary check during debugging, replace the direct python3.12
invocation with the venv-bound interpreter /app-root/.venv/bin/python3.12 to
avoid bypassing the virtualenv, but for merge delete the entire verification
lines and the TODO comment to avoid extra build overhead and potential release
failures.
Containerfile
Outdated
| # todo :remove this after testing | ||
| RUN echo "Verifying numpy installation..." && \ | ||
| python3.12 -c "import numpy; print('numpy version:', numpy.__version__)" && \ | ||
| echo "numpy verification successful" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, locate the Containerfile
fd -type f -name "Containerfile" -o -name "Dockerfile"Repository: lightspeed-core/lightspeed-stack
Length of output: 246
🏁 Script executed:
# Read the Containerfile to check lines 95-98
head -100 Containerfile | tail -10Repository: lightspeed-core/lightspeed-stack
Length of output: 386
🏁 Script executed:
# Get more context around lines 95-98
sed -n '80,110p' ContainerfileRepository: lightspeed-core/lightspeed-stack
Length of output: 1493
🏁 Script executed:
# Check if there are any build ARGs that could gate this
rg "ARG|COPY|FROM" Containerfile | head -20Repository: lightspeed-core/lightspeed-stack
Length of output: 555
Remove the temporary NumPy verification step before merge.
The TODO comment at Line 95 indicates this is temporary code. This verification layer should be removed before shipping to avoid unnecessary build overhead and prevent release build failures if the import breaks. The code uses python3.12 directly, which bypasses the venv in PATH; if you retain it temporarily, use /app-root/.venv/bin/python3.12 to target the installed wheel. Lines 95-98.
🧹 Suggested removal
-# todo :remove this after testing
-RUN echo "Verifying numpy installation..." && \
- python3.12 -c "import numpy; print('numpy version:', numpy.__version__)" && \
- echo "numpy verification successful"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # todo :remove this after testing | |
| RUN echo "Verifying numpy installation..." && \ | |
| python3.12 -c "import numpy; print('numpy version:', numpy.__version__)" && \ | |
| echo "numpy verification successful" |
🤖 Prompt for AI Agents
In `@Containerfile` around lines 95 - 98, Remove the temporary NumPy verification
RUN block that echoes and runs python3.12 -c "import numpy..." from the
Containerfile; if you must keep a temporary check during debugging, replace the
direct python3.12 invocation with the venv-bound interpreter
/app-root/.venv/bin/python3.12 to avoid bypassing the virtualenv, but for merge
delete the entire verification lines and the TODO comment to avoid extra build
overhead and potential release failures.
Signed-off-by: Haoyu Sun <hasun@redhat.com>
140df5d to
c1e0a39
Compare
|
ready to review and merge :D |
Description
fix missing lib problem for numpy wheel from RHOAI
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.